Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for group calls using MSC3401 #2553

Merged
merged 364 commits into from
Nov 7, 2022
Merged

Add support for group calls using MSC3401 #2553

merged 364 commits into from
Nov 7, 2022

Conversation

SimonBrandner
Copy link
Contributor

@SimonBrandner SimonBrandner commented Jul 29, 2022

Implementation of matrix-org/matrix-spec-proposals#3401


Here's what your changelog entry will look like:

✨ Features

SimonBrandner and others added 30 commits October 19, 2021 10:25
* Add group call events to EventType

Signed-off-by: Šimon Brandner <[email protected]>

* Use EventType instead of a const

Signed-off-by: Šimon Brandner <[email protected]>

* Make logging around sending group call member state event a bit better

Signed-off-by: Šimon Brandner <[email protected]>

* Fix m.calls elements being null

Signed-off-by: Šimon Brandner <[email protected]>
)

This avoids a race condition where the other side would first receive the to-device messages and only then the member state event which would result in the call being ignored

Signed-off-by: Šimon Brandner <[email protected]>
and assert that olm sessions are open to the destination devices
@dbkr dbkr marked this pull request as ready for review November 1, 2022 15:25
@dbkr dbkr requested a review from a team as a code owner November 1, 2022 15:25
Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've spot-checked a handful of files and aside from the comments raised here it looks like there's a lot of needless import changes in files - it'd be nice to clean those up for a clean diff, but not required.

A question would also be whether we want this to be a useful commit history and do a merge commit on the PR or hide it all behind a squashed commit.

.github/CODEOWNERS Outdated Show resolved Hide resolved
src/webrtc/callEventHandler.ts Outdated Show resolved Hide resolved
@dbkr
Copy link
Member

dbkr commented Nov 1, 2022

I'd vote for a merge commit - there's lots of useful history here in the individual PRs to the branch so it would be a shame to lose that.

@dbkr
Copy link
Member

dbkr commented Nov 1, 2022

I think the import changes are all valid as far as I can see, and are changing 'matrix' import to import directly from the original place instead.

@robintown
Copy link
Member

The import changes were necessary to prevent import loops, if I recall. It might not be possible to remove them, but I suppose we could put them in a separate PR if necessary.

@richvdh
Copy link
Member

richvdh commented Nov 4, 2022

sorry to weigh in, but

I'd vote for a merge commit - there's lots of useful history here in the individual PRs to the branch so it would be a shame to lose that.

I have a fairly strong counteropinion on this. Each git commit that gets merged to your project's mainline should be an atomic change that leaves the system in an operational state. There are hundreds of commits in this branch, many of them called things like "fix the tests". If the branch had been maintained with a coherent history then it might be reasonable to merge it, but that's not what's happened here.

"it would be a shame to lose this" is a spurious argument. Github keeps all of the commits comprising a squash-merged PR forever, so if at some point in the future we want to understand how this evolved, we can retrieve it.

I've previously written about this at https://matrix-org.github.io/synapse/latest/development/git.html#on-keeping-the-commit-history-clean.

@dbkr
Copy link
Member

dbkr commented Nov 4, 2022

Ah, the initial work on this branch was individual commits to it before we started PRing into it, which is annoying. We could bundle those commits up into a separate PR and reconstruct another branch, although that sounds like a way to lose another afternoon on this in addition to the hours of merging. Squash-merge it is, then.

@turt2live
Copy link
Member

(my counter-counter-argument for doing a merge commit would have been git blame support, but it is indeed a bit of a smell if we're considering a merge commit at all - normally that'd mean we should have had 300+ PRs. For this case though, any merge strategy seems not-great, so picking the one the button defaults to would be fine)

Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spot check looks fine

@dbkr dbkr merged commit df2b65f into develop Nov 7, 2022
@dbkr dbkr deleted the robertlong/group-call branch November 7, 2022 17:24
su-ex added a commit to SchildiChat/matrix-js-sdk that referenced this pull request Nov 25, 2022
* Make calls go back to 'connecting' state when media lost ([\matrix-org#2880](matrix-org#2880)).
* Add ability to send unthreaded receipt ([\matrix-org#2878](matrix-org#2878)).
* Add way to abort search requests ([\matrix-org#2877](matrix-org#2877)).
* sliding sync: add custom room subscriptions support ([\matrix-org#2834](matrix-org#2834)).
* webrtc: add advanced audio settings ([\matrix-org#2434](matrix-org#2434)). Contributed by @MrAnno.
* Add support for group calls using MSC3401 ([\matrix-org#2553](matrix-org#2553)).
* Make the js-sdk conform to tsc --strict ([\matrix-org#2835](matrix-org#2835)). Fixes matrix-org#2112 matrix-org#2116 and matrix-org#2124.
* Let leave requests outlive the window ([\matrix-org#2815](matrix-org#2815)). Fixes element-hq/element-call#639.
* Add event and message capabilities to RoomWidgetClient ([\matrix-org#2797](matrix-org#2797)).
* Misc fixes for group call widgets ([\matrix-org#2657](matrix-org#2657)).
* Support nested Matrix clients via the widget API ([\matrix-org#2473](matrix-org#2473)).
* Set max average bitrate on PTT calls ([\matrix-org#2499](matrix-org#2499)). Fixes element-hq/element-call#440.
* Add config option for e2e group call signalling ([\matrix-org#2492](matrix-org#2492)).
* Enable DTX on audio tracks in calls ([\matrix-org#2482](matrix-org#2482)).
* Don't ignore call member events with a distant future expiration date ([\matrix-org#2466](matrix-org#2466)).
* Expire call member state events after 1 hour ([\matrix-org#2446](matrix-org#2446)).
* Emit unknown device errors for group call participants without e2e ([\matrix-org#2447](matrix-org#2447)).
* Mute disconnected peers in PTT mode ([\matrix-org#2421](matrix-org#2421)).
* Add support for sending encrypted to-device events with OLM ([\matrix-org#2322](matrix-org#2322)). Contributed by @robertlong.
* Support for PTT group call mode ([\matrix-org#2338](matrix-org#2338)).
* Fix registration add phone number not working ([\matrix-org#2876](matrix-org#2876)). Contributed by @bagvand.
* Use an underride rule for Element Call notifications ([\matrix-org#2873](matrix-org#2873)). Fixes element-hq/element-web#23691.
* Fixes unwanted highlight notifications with encrypted threads ([\matrix-org#2862](matrix-org#2862)).
* Extra insurance that we don't mix events in the wrong timelines - v2 ([\matrix-org#2856](matrix-org#2856)). Contributed by @MadLittleMods.
* Hide pending events in thread timelines ([\matrix-org#2843](matrix-org#2843)). Fixes element-hq/element-web#23684.
* Fix pagination token tracking for mixed room timelines ([\matrix-org#2855](matrix-org#2855)). Fixes element-hq/element-web#23695.
* Extra insurance that we don't mix events in the wrong timelines ([\matrix-org#2848](matrix-org#2848)). Contributed by @MadLittleMods.
* Do not freeze state in `initialiseState()` ([\matrix-org#2846](matrix-org#2846)).
* Don't remove our own member for a split second when entering a call ([\matrix-org#2844](matrix-org#2844)).
* Resolve races between `initLocalCallFeed` and `leave` ([\matrix-org#2826](matrix-org#2826)).
* Add throwOnFail to groupCall.setScreensharingEnabled ([\matrix-org#2787](matrix-org#2787)).
* Fix connectivity regressions ([\matrix-org#2780](matrix-org#2780)).
* Fix screenshare failing after several attempts ([\matrix-org#2771](matrix-org#2771)). Fixes element-hq/element-call#625.
* Don't block muting/unmuting on network requests ([\matrix-org#2754](matrix-org#2754)). Fixes element-hq/element-call#592.
* Fix ICE restarts ([\matrix-org#2702](matrix-org#2702)).
* Target widget actions at a specific room ([\matrix-org#2670](matrix-org#2670)).
* Add tests for ice candidate sending ([\matrix-org#2674](matrix-org#2674)).
* Prevent exception when muting ([\matrix-org#2667](matrix-org#2667)). Fixes element-hq/element-call#578.
* Fix race in creating calls ([\matrix-org#2662](matrix-org#2662)).
* Add client.waitUntilRoomReadyForGroupCalls() ([\matrix-org#2641](matrix-org#2641)).
* Wait for client to start syncing before making group calls ([\matrix-org#2632](matrix-org#2632)). Fixes matrix-org#2589.
* Add GroupCallEventHandlerEvent.Room ([\matrix-org#2631](matrix-org#2631)).
* Add missing events from reemitter to GroupCall ([\matrix-org#2527](matrix-org#2527)). Contributed by @toger5.
* Prevent double mute status changed events ([\matrix-org#2502](matrix-org#2502)).
* Don't mute the remote side immediately in PTT calls ([\matrix-org#2487](matrix-org#2487)). Fixes element-hq/element-call#425.
* Fix some MatrixCall leaks and use a shared AudioContext ([\matrix-org#2484](matrix-org#2484)). Fixes element-hq/element-call#412.
* Don't block muting on determining whether the device exists ([\matrix-org#2461](matrix-org#2461)).
* Only clone streams on Safari ([\matrix-org#2450](matrix-org#2450)). Fixes element-hq/element-call#267.
* Set PTT mode on call correctly ([\matrix-org#2445](matrix-org#2445)). Fixes element-hq/element-call#382.
* Wait for mute event to send in PTT mode ([\matrix-org#2401](matrix-org#2401)).
* Handle other members having no e2e keys ([\matrix-org#2383](matrix-org#2383)). Fixes element-hq/element-call#338.
* Fix races when muting/unmuting ([\matrix-org#2370](matrix-org#2370)).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants